Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clarify upload docs #944

Merged
merged 8 commits into from
Jul 25, 2022
Merged

Clarify upload docs #944

merged 8 commits into from
Jul 25, 2022

Conversation

stevhliu
Copy link
Member

@stevhliu stevhliu commented Jul 7, 2022

This PR addresses #938.

About the third point, I think if we show upload_file first (also shown in the Quickstart but not everyone may land there first) then it'll be easier for users to immediately get more value from the doc.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 7, 2022 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

osanseviero
osanseviero reviewed Jul 8, 2022
View reviewed changes
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
@stevhliu
Copy link
Member Author

stevhliu commented Jul 8, 2022

Also added a section in the guide for upload_folder since the only mention of it is in the Reference section.

@stevhliu stevhliu mentioned this pull request Jul 15, 2022
Closed
@osanseviero osanseviero mentioned this pull request Jul 20, 2022
Closed
@julien-c
Copy link
Member

lol, I should have checked this PR before opening #947 🤦

Still worth it checking #947 to see if anything should be propagated back here, @stevhliu. Thanks!

@stevhliu
Copy link
Member Author

I reordered the methods according to the recommended way for uploading files while keeping @osanseviero's suggestion that we should introduce the more convenient upload_file and upload_folder first.

osanseviero
osanseviero reviewed Jul 20, 2022
View reviewed changes
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice! I left a couple of last comments, but seems in good state! Feel free to re-request review once the comments are addressed

docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
docs/source/how-to-upstream.mdx Outdated Show resolved Hide resolved
LysandreJik
LysandreJik approved these changes Jul 21, 2022
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @stevhliu

osanseviero
osanseviero approved these changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for working and iterating on this!

@osanseviero osanseviero merged commit faeeb28 into huggingface:main Jul 25, 2022
@stevhliu stevhliu deleted the docs-feedback branch July 25, 2022 18:01
@Wauplin Wauplin mentioned this pull request Aug 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NielsRogge NielsRogge NielsRogge left review comments

@LysandreJik LysandreJik LysandreJik approved these changes

@osanseviero osanseviero osanseviero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stevhliu @HuggingFaceDocBuilderDev @julien-c @osanseviero @LysandreJik @NielsRogge